Skip to content

Conversation

jsdt
Copy link
Contributor

@jsdt jsdt commented Sep 24, 2025

Description of Changes

This exposes client credentials in reducer calls for rust.

API and ABI breaking changes

API Changes:

The main API change is the addition of AuthCtx and the sender_auth in ReducerContext.

ABI Changes:

This adds two new functions jwt_len and get_jwt. These use st_connection_credentials to look up the credentials associated with a connection id. jwt_len can be used to figure out the size of the payload, so that get_jwt can provide a large enough buffer.

TODO: I assume I need to bump the ABI version in this PR.

Expected complexity level and risk

  1. This adds new ABI functions

Testing

I've done some manual testing with modified versions of the quickstart. We should add some examples that use the new API.

@jsdt jsdt requested a review from gefjon September 24, 2025 20:28
@Centril Centril self-requested a review September 25, 2025 09:59
Comment on lines 395 to 396
"spacetime_10.0"::get_jwt,
"spacetime_10.0"::jwt_len,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These should be in a new ABI version and module, spacetime_11.0. See comment in bindings_sys/src/lib.rs around line 20.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, looks like this should be spacetime_10.1, actually.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See #3294 for an example of adding a new host function with a new minor version. These can all go in 10.1 so long as we don't cut a release in between the two PRs.

@bfops bfops added the release-any To be landed in any release window label Sep 29, 2025
})
}

pub fn get_jwt(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs corresponding docs in the style of e.g., table_id_from_name including traps and errors (e.g., NOT_IN_TRANSACTION applies here).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated it to clarify that it traps for errors. Returning an integer error code doesn't seem very useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the design of the overall ABI that NOT_IN_TRANSACTION should be returned as a code and not a trap, and I think we should follow the general ABI design.

target_ptr: WasmPtr<u32>,
) -> RtResult<()> {
log::info!("Calling get_jwt");
Self::with_span(caller, AbiCall::GetJwt, |caller| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this could use cvt_ret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It probably could, but it using that function just seemed to make the error handling more difficult to reason about.

Copy link
Contributor

@Centril Centril left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like there were also stuff from my previous review that was not addressed.

})
}

pub fn get_jwt(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is the design of the overall ABI that NOT_IN_TRANSACTION should be returned as a code and not a trap, and I think we should follow the general ABI design.

///
/// Traps if:
///
/// - `connection_id` does not point to a valid little-endian `ConnectionId`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// - `connection_id` does not point to a valid little-endian `ConnectionId`.
/// - `connection_id` does not point to a valid little-endian `ConnectionId`.
/// - `target_ptr` is NULL or `target_ptr[..size_of::<u32>()]` is not in bounds of WASM memory.
/// - The `ByteSourceId` to be written to `target_ptr` would overflow [`u32::MAX`].

(and in bindings-sys too)

Comment on lines 532 to 538
let mut source: BytesSource = BytesSource::INVALID;
unsafe {
raw::get_jwt(connection_id.as_le_byte_array().as_ptr(), &mut source);
};
if source == BytesSource::INVALID {
return None;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let mut source: BytesSource = BytesSource::INVALID;
unsafe {
raw::get_jwt(connection_id.as_le_byte_array().as_ptr(), &mut source);
};
if source == BytesSource::INVALID {
return None;
}
let source = sys::get_jwt(connection_id.as_le_byte_array())?;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-any To be landed in any release window

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants